Skip to content

feat(shorebird_cli): add patches rollback / rollforward#3750

Open
bdero wants to merge 5 commits into
mainfrom
bdero/cli-rollback-patches
Open

feat(shorebird_cli): add patches rollback / rollforward#3750
bdero wants to merge 5 commits into
mainfrom
bdero/cli-rollback-patches

Conversation

@bdero
Copy link
Copy Markdown
Member

@bdero bdero commented May 8, 2026

Summary

Adds CLI surface for the rollback and rollforward patch operations that
were previously dashboard-only. Both invocations are single network calls
gated by a pre-flight is_rolled_back check so the no-op case prints a
helpful message instead of relying on the server's 304 response.

shorebird patches rollback    --release-version 1.0.0+1 --patch-number 1
shorebird patches rollforward --release-version 1.0.0+1 --patch-number 1

Both accept --app-id / --flavor and the global --json flag.

Why

The HTTP endpoints have existed for a while, but the only way to invoke
them today is through the dashboard. Two recent uses of these operations
have been workflow-friction:

  1. Reproducing bug: checkForUpdate reports upToDate after patch-to-release rollback (regression of #3206) #3728 end-to-end against a real
    device required scripting raw curl calls against api.shorebird.dev
    in a side repo, because the dashboard isn't scriptable.
  2. CI pipelines that want to roll back a bad patch automatically (canary
    regression detected, kill-switch flipped, etc.) have to roll their own
    HTTP client + token handling.

A CLI command is the natural shape for both.

Implementation

Three layers, mirroring the existing patches set-track shape end-to-end.

Layer 1 — shorebird_code_push_client

New rollbackPatch / rollforwardPatch methods on CodePushClient that
POST to:

/apps/<id>/releases/<id>/patches/<id>/rollback
/apps/<id>/releases/<id>/patches/<id>/rollforward

Both treat HTTP 304 Not Modified (the server's "already in target state"
response — see code_push_server/routes/.../rollback.dart) as success, so
the call is idempotent at the client level too.

Layer 2 — code_push_client_wrapper

Wraps each with the standard logger.progress + _handleErrorAndExit
pattern, matching promotePatch. An optional patchNumber parameter
feeds into the progress label since the caller usually has it on hand
("Rolling back patch 5" reads better than "Rolling back patch").

Layer 3 — RollbackCommand / RollforwardCommand

Registered under the existing patches command group. Each pre-checks
is_rolled_back from getReleasePatches before issuing the POST so:

  • Trying to roll back an already-rolled-back patch prints
    Patch 1 is already rolled back and exits with usage error, instead of
    silently 304-ing and printing success.
  • Same shape for rollforward against an already-active patch.

--json envelope on success:

{"release_version":"1.0.0+1","patch_number":1,"action":"rollback"}

(action distinguishes the two commands for callers that machine-read.)

Naming + flag conventions

  • Command names rollback / rollforward match the HTTP endpoint paths,
    the server-side filenames, and the dashboard JS client's rollBackPatch
    / rollForwardPatch methods. The dashboard UI labels them "Rollback"
    and "Roll Forward" but everything below the UI uses the single-token
    forms.
  • Flags use --release-version (via CommonArguments.releaseVersionArg)
    and --patch-number, matching patches info (feat(shorebird_cli): add patches list/info; --json for set-track #3740). The older
    patches set-track uses --release / --patch which is a pre-existing
    inconsistency; this PR doesn't propagate it into new commands.

Test plan

  • dart analyze clean across all changed files.
  • dart test green:
    • code_push_client_test.dart — request shape, 204 + 304 success paths,
      unknown-error path, structured-error-response path. 8 cases.
    • code_push_client_wrapper_test.dart — progress success + failure for
      both verbs, optional patchNumber label. 5 cases.
    • rollback_command_test.dart / rollforward_command_test.dart
      validation failure, --app-id override, missing patch,
      already-in-target-state, success, fetch-failure (text + json),
      POST-failure (text + json), JSON envelope shape on each branch.
      30 cases.
  • Smoke-tested end-to-end against the real api.shorebird.dev while
    reproducing bug: checkForUpdate reports upToDate after patch-to-release rollback (regression of #3206) #3728: rollback returns 204,
    device picks up the rollback signal on the next poll, then rollforward
    returns 204 and the device's auto-update thread either reinstalls
    (rc2 / fixed) or refuses (rc1 / broken) the patch as expected.
  • shorebird patches --help lists the new subcommands; each
    --help renders the flag table with (mandatory) annotations.

Refs

@bdero bdero added enhancement New feature or request cli The issue pertains to the shorebird CLI labels May 8, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@eseidel eseidel requested a review from nickshorebird May 8, 2026 20:45
@bdero bdero mentioned this pull request May 8, 2026
3 tasks
bdero added 4 commits May 8, 2026 13:59
Adds CLI surface for the rollback and rollforward operations that were
previously dashboard-only. Mirrors the shape of the existing
`patches set-track` command (release + patch lookup, --json envelope,
deferred channel/state checks before the network call).

  shorebird patches rollback --release 1.0.0+1 --patch 1
  shorebird patches rollforward --release 1.0.0+1 --patch 1

Both commands also accept --app-id / --flavor for callers that don't
have a shorebird.yaml on hand (CI scripts, monorepos with multiple
apps, etc.).

Three layers of changes:

1. shorebird_code_push_client: new `rollbackPatch` and `rollforwardPatch`
   methods on CodePushClient. Each POSTs to
   `/apps/<id>/releases/<id>/patches/<id>/{rollback,rollforward}` and
   treats HTTP 304 (server's "already in target state" response) as
   a no-op success so the call is idempotent.

2. shorebird_cli code_push_client_wrapper: wraps each with the standard
   logger.progress + _handleErrorAndExit pattern. Optional patchNumber
   parameter feeds into the progress label since the caller usually has
   it on hand.

3. shorebird_cli commands: RollbackCommand and RollforwardCommand,
   registered under the existing `patches` command group. Pre-checks
   `is_rolled_back` from getReleasePatches before issuing the POST so
   the no-op case prints a helpful message instead of relying on the
   server's 304 path.

Tests:

- code_push_client_test.dart: request shape, 204 + 304 success paths,
  unknown-error path, structured-error-response path.
- code_push_client_wrapper_test.dart: progress success/failure, optional
  patchNumber label.
- rollback_command_test.dart, rollforward_command_test.dart: full
  matrix mirroring set_track_command_test — validation failure,
  --app-id override, missing patch, already-in-target-state, success,
  fetch-failure (text + json), API-failure (text + json), and the json
  envelope shape on each branch.

Verified end-to-end: `shorebird patches --help` lists the new
subcommands; `shorebird patches rollback --help` and
`shorebird patches rollforward --help` both render the expected usage.
Word appears in a comment on rollforwardPatch in code_push_client_wrapper —
'the server resends the same patch artifact on rollforward'. Same fix as
shorebirdtech/updater#356 needed in this repo.
@bdero bdero force-pushed the bdero/cli-rollback-patches branch from 1535ce4 to 945e8e7 Compare May 8, 2026 21:00
Copy link
Copy Markdown
Contributor

@nickshorebird nickshorebird left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM overall - Two nits for behavior. Thanks for putting this together.

if (isJsonMode) {
emitJsonError(
code: JsonErrorCode.usageError,
message: 'Patch $patchNumber is already rolled back.',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take it or leave it - End state is the same is the same on success or hitting this route. Returning a non zero exit suggests that something went wrong even though the end state is correct. Consider adding some flag IE --require-change, I am awful at naming to please feel free to change it, if you want the behavior to be there on default or non default. Changing to this would also match other CLI tool behaviors. This is in line w/ current set-track behavior hence take it or leave it.

Comment on lines +143 to +145
'release_version': releaseVersion,
'patch_number': patchNumber,
'action': 'rollback',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take it or leave it - Consider printing the patch object similar to patches info on rollback and rollforward. Would reduce the need to do follow up patches info call.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cli The issue pertains to the shorebird CLI enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants